-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate TextControl styles from SCSS to CSS-in-JS #18652
Migrate TextControl styles from SCSS to CSS-in-JS #18652
Conversation
@@ -0,0 +1,46 @@ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These mixins implement a consistent spacing system for wordpress
|
||
function BaseControl( { id, label, hideLabelFromVision, help, className, children } ) { | ||
return ( | ||
<div className={ classnames( 'components-base-control', className ) }> | ||
<div className="components-base-control__field"> | ||
<Control className={ classnames( 'components-base-control', className ) }> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping the "unused" classnames for backwards compatibility (they're also used by components that compose this component).
.components-base-control__field { | ||
margin-bottom: $grid-size; | ||
|
||
.components-panel__row & { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved this style to the Panel
component.
@@ -1,4 +0,0 @@ | |||
.components-text-control__input { | |||
width: 100%; | |||
padding: 6px 8px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6px
is changed to 4px
for consistency.
@jameslnewell This is awesome! Thank you for taking the initiative to do this :). Since we're starting to add/refactor things with Emotion, I think we have to determine a way of naming/placing For Card, I had the styles under That being said, I'm open to any convention :). As long as we're consistent. Would love your thoughts |
@ItsJonQ I've created a seperate issue to continue the discussion around file naming convention and structure (based on previous conversations it is likely a lengthy conversation and be a distraction from the purpose of this PR). When a decision is made, I'll come back and refactor any CSS-in-JS components that have made it into the repository before that decision is made. |
use spacing mixins for remaining spacing rules move panel styling to the panel use text() mixin fix use of rgba autofix linting fix snapshots moved files around and reused existing reduceMotion function changed style file structure and naming
Closing this one. |
Description
I've migrated the
TextControl
styles from SCSS to CSS-in-JS.The reasons I wish to migrate the
TextControl
styles to CSS-in-JS are:gutenberg
)I've introduced a number of mixins that can be re-used throughout the package as more components begin to use CSS-in-JS e.g. mixins for consistent spacing values. The mixins aren't exposed by the package, but it would likely be useful to expose them once CSS-in-JS is a more accepted direction.
This PR shouldn't introduce any breaking changes unless there is someone using the SCSS seperate from the React component.
There are a few minor visual changes bringing consistency with the proposed typography and spacing systems e.g. padding changed from
6px
to4px
and using the standard typography mixins (which aren't responsive like input was).How has this been tested?
I commented out the style imports (here, here and here) for the storybook (because the base styles have higher specificity) and created a story to confirm the
TextControl
continues to look the same (minus intentional changes).Screenshots
Base Control (Before):
Base Control (After):
Text Control (Before):
Text Control (After):
Text Control (After & with no base-styles):
Types of changes
Fixing issues in using the components without the reset - refactoring existing code with no expected breaking changes and no additional API surface area.
Checklist: